CNTRLPLANE-2834: add e2e test for etcd snapshot backup method on AWS - #8231
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new e2e test suite and helpers to validate etcd snapshot backup and restore for AWS HostedClusters. The suite sets the OADP ConfigMap to use etcd snapshots, runs an OADP backup, waits for HCPEtcdBackup completion and captures the SnapshotURL, breaks the HostedCluster (preserving machines), runs an OADP restore, waits for restore completion (including post-restore hooks), and verifies control-plane readiness, HostedCluster restore metadata, and etcd init-container logs. New helpers provide name matching, condition polling, and etcd-init log parsing; tests validate those helpers. Sequence DiagramsequenceDiagram
participant Test as Test Suite
participant K8s as Kubernetes API
participant OADP as OADP Controller
participant HCPBackup as HCPEtcdBackup Resource
participant HC as HostedCluster
participant Etcd as etcd-0 Pod
Note over Test,K8s: Setup
Test->>K8s: Update hypserhift-oadp-plugin-config (etcdBackupMethod=snapshot)
Test->>OADP: Create OADP Backup (UseEtcdSnapshot=true)
OADP->>HCPBackup: Create HCPEtcdBackup
Test->>HCPBackup: Poll for BackupCompleted=True
HCPBackup-->>Test: BackupCompleted + SnapshotURL
Note over Test,HC: Verification & Break
Test->>HC: Verify LastSuccessfulEtcdBackupURL == SnapshotURL
Test->>HC: Break HostedCluster (preserve machines)
Note over Test,OADP: Restore
Test->>OADP: Create OADP Restore (UseEtcdSnapshot=true)
OADP->>K8s: Perform restore, run postRestoreHook
Test->>OADP: Wait for restore completion
OADP-->>Test: Restore complete
Note over Test,Etcd: Post-restore checks
Test->>K8s: Check HC Spec.Etcd.Managed.Storage.RestoreSnapshotURL
Test->>Etcd: Stream etcd-init logs
Etcd-->>Test: Logs (restoring/restored)
Test->>Test: Parse logs and validate restore markers
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
area/testing
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/v2/tests/backup_restore_test.go`:
- Around line 478-503: The assertions currently only check for non-empty values
and can pick up leftover resources; modify the test to tie results to the
snapshot created in this run by recording the expected snapshot URL (from the
HCPEtcdBackup that corresponds to the current backupName) and using Eventually
to wait for that specific URL to appear: list HCPEtcdBackup resources
(HCPEtcdBackupList) and locate the item whose metadata.name equals backupName,
capture its Status.SnapshotURL, then use Eventually (with testCtx.Context/test
timeout) to wait until HostedCluster (get via testCtx.MgmtClient.Get into
hostedCluster) has Status.LastSuccessfulEtcdBackupURL equal to that captured
SnapshotURL and that hostedCluster.Status.RestoreSnapshotURL[0] (or the
appropriate element) also equals it, failing if not matched; replace the generic
non-empty Expect checks with equality assertions against the captured snapshot
URL.
- Around line 379-421: The ConfigMap override and DeferCleanup are currently
inside the test body so DeferCleanup runs too early and also discards any
pre-existing state; move the ConfigMap creation/patch logic that uses
testCtx.MgmtClient.Get/Create/Update and the cmKey/cm variable into a
BeforeAll() (or call DeferCleanup/RegisterCleanup from BeforeAll) so the
"etcdBackupMethod: etcdSnapshot" stays in place for subsequent specs, and when
setting up capture and store the original ConfigMap state (nil if not present,
or copy of configMap.Data) so the cleanup restores the previous data instead of
unconditionally deleting the key or ConfigMap. Ensure cleanup code uses the
saved original value to either restore the map key, restore the whole ConfigMap
data, or delete the ConfigMap only if it did not exist before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b341070-49af-4c7f-947e-7ea2861bb70a
📒 Files selected for processing (1)
test/e2e/v2/tests/backup_restore_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8231 +/- ##
=======================================
Coverage 36.08% 36.08%
=======================================
Files 767 767
Lines 93486 93486
=======================================
Hits 33737 33737
Misses 57042 57042
Partials 2707 2707 🚀 New features to boost your workflow:
|
mgencur
left a comment
mgencur
left a comment
There was a problem hiding this comment.
Left some comments.
Let's discuss if this should be a standalone test or it can be merged with the other test.
| Expect(err).NotTo(HaveOccurred()) | ||
| } | ||
|
|
||
| DeferCleanup(func() { |
There was a problem hiding this comment.
Hmm. In Ginkgo, DeferCleanup registered inside an It block runs after that It completes (like an AfterEach scoped to that specific test).
So, it will probably delete the configmap before we actually do the backup/restore procedure.
Should this be moved to AfterAll?
| }) | ||
|
|
||
| Context("VerifyRestoreSnapshotURL", func() { | ||
| It("should have restoreSnapshotURL set on HostedCluster after restore", func() { |
There was a problem hiding this comment.
Could this It be moved under ContextPostRestoreControlPlane, next to "should have control plane healthy after restore". That's where control plane validations are supposed to be.
08e6736 to
6f70fe5
Compare
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/v2/tests/backup_restore_test.go (1)
397-416: FreezetestCtxbefore registering cleanup.The cleanup closes over
testCtx, which is reassigned in theBeforeEachat line 420. Go closures capture variables by reference, so the cleanup will use whatevertestCtxpoints to when it runs, not necessarily the same context that applied the override. Capture the management client and context into local variables before callingDeferCleanupto ensure cleanup operates on the same context as setup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/v2/tests/backup_restore_test.go` around lines 397 - 416, The DeferCleanup closure closes over the mutable testCtx (used later in BeforeEach), so freeze the exact client and context before registering cleanup: capture testCtx.MgmtClient and testCtx.Context into local variables (e.g., mgmtClient, ctx) and use those instead of testCtx inside the DeferCleanup body (also reference cmKey, cmExisted, originalData as before), calling mgmtClient.Get/Delete/Update with ctx to ensure the cleanup operates on the same resources that were modified during setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/v2/tests/backup_restore_test.go`:
- Around line 397-416: The DeferCleanup closure closes over the mutable testCtx
(used later in BeforeEach), so freeze the exact client and context before
registering cleanup: capture testCtx.MgmtClient and testCtx.Context into local
variables (e.g., mgmtClient, ctx) and use those instead of testCtx inside the
DeferCleanup body (also reference cmKey, cmExisted, originalData as before),
calling mgmtClient.Get/Delete/Update with ctx to ensure the cleanup operates on
the same resources that were modified during setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e3a3bf1d-f886-4bb7-9754-af6d02c856b1
📒 Files selected for processing (1)
test/e2e/v2/tests/backup_restore_test.go
|
/jira refresh |
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/assign @jparrill |
|
Hey @sdminonne some things:
|
6f70fe5 to
c908d77
Compare
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/v2/backuprestore/etcd_snapshot.go`:
- Around line 125-152: The test currently accumulates the entire etcd-init
output into logLines and prints every line; instead limit retained logs to a
bounded tail (e.g., a fixed-size ring buffer or slice that keeps only the last N
lines) while scanning logStream using scanner, set foundDownload/foundRestore as
you do now, and only emit the retained tail to logger if scanner.Err() is
non-nil or if either foundDownload or foundRestore is false (failure path);
reference the scanner, logStream, foundDownload, foundRestore, logLines and
logger symbols when making the change so you replace the unbounded append and
unconditional logging with a bounded buffer + conditional logging on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a995ea2e-8ef5-45b1-bd96-b48c50a9f48e
📒 Files selected for processing (3)
test/e2e/karpenter_test.gotest/e2e/v2/backuprestore/etcd_snapshot.gotest/e2e/v2/tests/backup_restore_test.go
✅ Files skipped from review due to trivial changes (1)
- test/e2e/karpenter_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/v2/tests/backup_restore_test.go
mgencur
left a comment
mgencur
left a comment
There was a problem hiding this comment.
One minor comment, otherwise LGTM
| armNodeLabels := map[string]string{ | ||
| karpenterv1.NodePoolLabelKey: armNodePool.Name, | ||
| "kubernetes.io/arch": "arm64", | ||
| "kubernetes.io/arch": "arm64", |
There was a problem hiding this comment.
Nit: This change is redundant?
jparrill
left a comment
jparrill
left a comment
There was a problem hiding this comment.
Some comments:
-
Claude detected some duplicated functions on some places, maybe we can create some functions:
backup_restore_test.go:124-136-- BeforeEachbackup_restore_test.go:138-163-- ContextPreBackupControlPlane bodybackup_restore_test.go:261-265-- ContextBreakControlPlane bodybackup_restore_test.go:268-291-- ContextRestore structurebackup_restore_test.go:293-312-- ContextPostRestoreControlPlane core health check
-
Functions like:
func ValidateBeforeEach(testCtx *internal.TestContext) { ... }func ValidatePreBackupControlPlane(testCtx *internal.TestContext, excludeWorkloads []string) []util.Condition { ... }func ValidatePostRestoreControlPlane(testCtx *internal.TestContext, excludeWorkloads []string, expectedConditions []util.Condition) { ... }
Declare some const:
- "etcd-init" (container name)
- "etcd-0" (pod name)
- "snapshot downloaded successfully"
- "snapshot restore succeeded"
- "etcd snapshot restored successfully"
The pr is getting a good shape :). Thanks for that
| line := scanner.Text() | ||
| logLines = append(logLines, line) | ||
| lower := strings.ToLower(line) | ||
| if strings.Contains(lower, "snapshot downloaded successfully") { |
There was a problem hiding this comment.
Humm, I think we need to adjust this, curl shows progress bars, no success echo
| if strings.Contains(lower, "snapshot downloaded successfully") { | ||
| foundDownload = true | ||
| } | ||
| if strings.Contains(lower, "snapshot restore succeeded") || strings.Contains(lower, "etcd snapshot restored successfully") { |
There was a problem hiding this comment.
Humm, I think we need to adjust this, current output shows:
...restoring snapshot...
...restored snapshot...
Full sample here, maybe not exactly as it shows in the reality but pretty close:
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 46.8M 100 46.8M 0 0 34.4M 0 0:00:01 0:00:01 --:--:-- 34.4M
/bin/sh: line 13: file: command not found
INFO: using etcdutl (etcd 3.6+)
+----------+----------+------------+------------+---------+
| HASH | REVISION | TOTAL KEYS | TOTAL SIZE | VERSION |
+----------+----------+------------+------------+---------+
| 5643d825 | 578454 | 3209 | 49 MB | 3.6.0 |
+----------+----------+------------+------------+---------+
2026-04-13T07:33:20Z info snapshot/v3_snapshot.go:305 restoring snapshot {"path": "/tmp/snapshot", "wal-dir": "/var/lib/data/member/wal", "data-dir": "/var/lib/data", "snap-dir":
"/var/lib/data/member/snap", "initial-memory-map-size": 10737418240}
2026-04-13T07:33:20Z info bbolt backend/backend.go:203 Opening db file (/var/lib/data/member/snap/db) with mode -rw------- and with options: {Timeout: 0s, NoGrowSync: false, NoFreelistSync: true,
PreLoadFreelist: false, FreelistType: , ReadOnly: false, MmapFlags: 8000, InitialMmapSize: 10737418240, PageSize: 0, NoSync: false, OpenFile: 0x0, Mlock: false, Logger: 0xc0000661a0}
2026-04-13T07:33:20Z info bbolt bbolt@v1.4.3/db.go:322 Opening bbolt db (/var/lib/data/member/snap/db) successfully
2026-04-13T07:33:20Z info schema/membership.go:138 Trimming membership information from the backend...
2026-04-13T07:33:20Z info bbolt backend/backend.go:203 Opening db file (/var/lib/data/member/snap/db) with mode -rw------- and with options: {Timeout: 0s, NoGrowSync: false, NoFreelistSync: true,
PreLoadFreelist: false, FreelistType: , ReadOnly: false, MmapFlags: 8000, InitialMmapSize: 10737418240, PageSize: 0, NoSync: false, OpenFile: 0x0, Mlock: false, Logger: 0xc000066138}
2026-04-13T07:33:20Z info bbolt bbolt@v1.4.3/db.go:322 Opening bbolt db (/var/lib/data/member/snap/db) successfully
2026-04-13T07:33:20Z info membership/cluster.go:424 added member {"cluster-id": "cdf818194e3a8c32", "local-member-id": "0", "added-peer-id": "8e9e05c52164694d", "added-peer-peer-urls":
["http://localhost:2380"], "added-peer-is-learner": false}
2026-04-13T07:33:20Z info bbolt backend/backend.go:203 Opening db file (/var/lib/data/member/snap/db) with mode -rw------- and with options: {Timeout: 0s, NoGrowSync: false, NoFreelistSync: true,
PreLoadFreelist: false, FreelistType: , ReadOnly: false, MmapFlags: 8000, InitialMmapSize: 10737418240, PageSize: 0, NoSync: false, OpenFile: 0x0, Mlock: false, Logger: 0xc000066210}
2026-04-13T07:33:20Z info bbolt bbolt@v1.4.3/db.go:322 Opening bbolt db (/var/lib/data/member/snap/db) successfully
2026-04-13T07:33:20Z info snapshot/v3_snapshot.go:333 restored snapshot {"path": "/tmp/snapshot", "wal-dir": "/var/lib/data/member/wal", "data-dir": "/var/lib/data", "snap-dir":
"/var/lib/data/member/snap", "initial-memory-map-size": 10737418240}
| } | ||
|
|
||
| if !foundDownload { | ||
| return fmt.Errorf("etcd-init logs do not contain 'snapshot downloaded successfully'; snapshot download may have failed") |
There was a problem hiding this comment.
Check the comments above.
I would also store the expected msgs in constants
| return fmt.Errorf("etcd-init logs do not contain 'snapshot downloaded successfully'; snapshot download may have failed") | ||
| } | ||
| if !foundRestore { | ||
| return fmt.Errorf("etcd-init logs do not contain 'snapshot restore succeeded' or 'etcd snapshot restored successfully'; restore may have failed") |
There was a problem hiding this comment.
Check the comments above.
I would also store the expected msgs in constants
| } | ||
| if !foundRestore { | ||
| return fmt.Errorf("etcd-init logs do not contain 'snapshot restore succeeded' or 'etcd snapshot restored successfully'; restore may have failed") | ||
| } |
There was a problem hiding this comment.
I would add another check to make sure the ETCD restoration didn't fail, looking for:
/var/lib/data not empty, not restoring snapshot
| if cm.Data == nil { | ||
| cm.Data = map[string]string{} | ||
| } | ||
| cm.Data["etcdBackupMethod"] = "etcdSnapshot" |
|
@sdminonne: This pull request references CNTRLPLANE-2834 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/v2/backuprestore/etcd_snapshot.go`:
- Around line 96-100: Update MatchesHCPEtcdBackupName to require the trailing
hyphen so it matches the documented pattern "oadp-<BackupName>-<random>" (use
HCPEtcdBackupNamePrefix + oadpBackupName + "-" when checking the prefix) and add
the suggested regression test case to TestMatchesHCPEtcdBackupName (the case
where hcpEtcdBackupName "oadp-mycluster-mynamespace-abc1234-xyz78" should not
match oadpBackupName "mycluster-mynamespace-abc123") to ensure names that only
share a prefix do not falsely match.
In `@test/e2e/v2/tests/backup_restore_test.go`:
- Around line 505-523: The test prints full snapshot URLs via
GinkgoWriter.Printf, which can leak presigned credentials; update the prints in
the HCPEtcdBackup checks (the block using
backuprestore.MatchesHCPEtcdBackupName, variable snapshotURL and backup.Name) to
avoid emitting backup.Status.SnapshotURL — instead log only the backup resource
name and a redacted marker (e.g. "[REDACTED_SNAPSHOT_URL]") or omit the URL
entirely; apply the same change to the other similar GinkgoWriter.Printf
occurrences that reference snapshotURL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: eeb9358e-566d-4b79-a806-3411edb767e8
📒 Files selected for processing (3)
test/e2e/v2/backuprestore/etcd_snapshot.gotest/e2e/v2/backuprestore/etcd_snapshot_test.gotest/e2e/v2/tests/backup_restore_test.go
|
Let's run |
…used restoreName variable Replace the slice-shift tail buffer in parseEtcdInitLogs with a fixed-size ring buffer so old strings are overwritten in place and become eligible for GC immediately. Remove restoreName from suite-level var blocks in both test suites since it is only used locally within the restore It block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace EtcdSnapshotBackupOptions/EtcdSnapshotRestoreOptions helpers with UseEtcdSnapshot field on the option structs, letting the CLI handle all etcd snapshot mode settings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…of exact match The restoreSnapshotURL in the HostedCluster spec contains a presigned HTTPS URL, which differs from the original S3 URL stored in HCPEtcdBackup.Status.SnapshotURL. Replace the Equal(snapshotURL) assertion with a non-empty check so the test no longer hangs waiting for two structurally different URLs to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ation
Replace single-shot ValidateControlPlane{Deployments,StatefulSets}Readiness
calls with WaitFor variants using a 5-minute timeout. This avoids transient
failures when a control plane workload is mid-rollout at the time the
pre-backup check runs.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dation PostBackupControlPlane used instant Validate* checks that fail on transient etcd unavailability after an OADP backup. Replace with WaitFor* polling (5 min timeout) matching PreBackupControlPlane and PostRestoreControlPlane. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…apshot backup The CI OADP setup creates the DataProtectionApplication without the hypershift plugin in its defaultPlugins, causing the etcd snapshot backup to complete with phase=PartiallyFailed. Add self-setup logic in the BackupRestoreEtcdSnapshot BeforeAll to append the hypershift plugin to the DPA if missing, wait for Velero to restart, and restore the original plugins during cleanup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hift plugin After adding the hypershift plugin to the DPA defaultPlugins, waiting only for the Velero pod to restart is insufficient. The OADP operator must also reconcile the DPA and set the Reconciled=True condition. Without this, the backup command's VerifyDPAStatus check rejects the backup with "no ready DataProtectionApplication found". Add ensureDPAReconciled to the poll loop so EnsureDPAHypershiftPlugin waits for both Velero readiness and DPA reconciliation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
716bb63 to
44bd4e7
Compare
…race After adding the hypershift plugin to the DPA spec, the poll used immediate=true which checked Reconciled=True before the OADP controller had time to process the spec change. The stale status from the previous reconciliation satisfied the check instantly, and moments later the controller cleared the condition while re-reconciling — causing the backup command to fail with "no ready DataProtectionApplication found". Switch to immediate=false so the first check happens after the 10s poll interval, giving the controller time to detect and process the change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
44bd4e7 to
62ea67a
Compare
|
/test e2e-v2-aws-backuprestore |
…le timeout The EnsureDPAHypershiftPlugin function was timing out after 5 minutes without any diagnostic information about why the Velero pod or DPA failed to become ready. Increase the timeout to 10 minutes to accommodate slow CI environments and add per-poll logging plus a diagnostic snapshot on failure that captures Velero pod status (phase, container readiness, restart count, waiting/terminated reasons) and DPA conditions (type, status, reason, message). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test e2e-v2-aws-backuprestore |
|
Now let me find the exact failure testcase in the JUnit and look at the test source code: |
… defaultPlugins EnsureDPAHypershiftPlugin only checked defaultPlugins for "hypershift" but the CI setup step configures the plugin via customPlugins with name "hypershift-oadp-plugin". Adding "hypershift" to defaultPlugins when it already exists in customPlugins causes the OADP operator to generate duplicate init containers named "hypershift-oadp-plugin" in the velero Deployment, which Kubernetes rejects with a validation error. Now the function also checks customPlugins for an entry named "hypershift-oadp-plugin" and treats it as a no-op if found. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test e2e-v2-aws-backuprestore |
Rerunning then |
|
/test e2e-v2-aws-backuprestore |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/lgtm |
|
Scheduling tests matching the |
|
/verified by E2E |
openshift-ci-robot
added
the
verified
|
@jparrill: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@sdminonne: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Add a new BackupRestoreEtcdSnapshot test that exercises the alternative etcdSnapshot backup flow via the OADP hypershift plugin ConfigMap. The test configures the plugin, creates a backup with snapshotMoveData=false, verifies HCPEtcdBackup snapshotURL and HostedCluster lastSuccessfulEtcdBackupURL, then performs a full break/restore cycle and validates restoreSnapshotURL is set on the restored HostedCluster.
Fixes: https://redhat.atlassian.net/browse/CNTRLPLANE-2834
Summary by CodeRabbit